Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements periodic pruning of the InnerForest in-memory data structures to bound memory growth, addressing issue #1175 about pruning account storage maps and vault tables.
Changes:
- Added pruning mechanism that retains only the last 50 blocks (configurable via
HISTORICAL_BLOCK_RETENTIONconstant) of account vault and storage map data - Introduced block-indexed tracking structures (
vault_roots_by_blockandstorage_slots_by_block) to efficiently identify entries eligible for pruning - Refactored
state/mod.rsto cacheblock.body()calls and improve code readability
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/store/src/inner_forest/mod.rs | Added pruning logic with new tracking data structures, HISTORICAL_BLOCK_RETENTION constant, and helper methods to prune old vault and storage map roots |
| crates/store/src/inner_forest/tests.rs | Added comprehensive test coverage for pruning functionality, including edge cases like empty forest, young chains, boundary conditions, and multiple accounts/slots |
| crates/store/src/state/mod.rs | Refactored to cache block.body() result and renamed block_data to block_bytes for clarity; split forest write lock acquisition into separate line |
| crates/store/src/db/tests.rs | Renamed test function to follow consistent naming convention using db_roundtrip_ prefix |
| CHANGELOG.md | Added enhancement entry for periodic cleanup feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 81 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
72dad48 to
a3be45b
Compare
|
Found one more bug, when using RPC with response The large changeset is mostly due to tests added. |
igamigo
left a comment
There was a problem hiding this comment.
Overall LGTM, but left a couple of comments. Not sure if they are fully correct (or if they land within the scope of this PR exactly), but worth reviewing for the long term at least.
|
@bobbinth it looks like the |
35b80c3 to
098c805
Compare
3e0f462 to
e2c5289
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0e269cb to
68b4b04
Compare
…r-forest-for-main
Important
Targeting
mainWhat
Does cleanup the
InnerForest, both the lookup tables/maps and the actualSmtForest.Required for bounding the in-memory size growth.
Context
Related #1175
How
There are a few requirements:
*_refcounttablesCaveats
::AllEntries, theSmtProofcontains the relevantSmtLeafitselfSmtForestfor a specific key